openapi3: store field locations in a slice, not a map - #25
Draft
reuvenharrison wants to merge 6 commits into
Draft
openapi3: store field locations in a slice, not a map#25reuvenharrison wants to merge 6 commits into
reuvenharrison wants to merge 6 commits into
Conversation
reuvenharrison
marked this pull request as draft
August 1, 2026 21:39
reuvenharrison
added a commit
to oasdiff/oasdiff
that referenced
this pull request
Aug 1, 2026
Must be removed before merge. This PR adapts to Origin.Fields becoming a slice, so it cannot compile against any published kin until #25 lands upstream and is released. Without the pin CI is red for a reason that says nothing about whether the adaptation is correct; with it, a green run means exactly that.
Origin.Fields was map[string]Location. A collection carries a handful of scalar fields, but a Go map allocates a whole bucket per collection whatever it holds, so the map dominated the retained size of a parsed document: on a 22 MB spec, inserting into it accounted for 294 MB of the 715 MB retained, and the maps themselves for another 22 MB. Replace it with FieldLocations, a slice in document order. Each Location already carries its Name, so the lookup key costs nothing extra, and lookup is a linear scan, which beats a map at these sizes. Retained document size drops 47% at both sizes measured: 5 MB spec: 131 MB -> 69 MB 22 MB spec: 536 MB -> 282 MB Peak heap during load is unchanged, since it is dominated by the transient yaml parse tree rather than by what is retained. Get(name) returns a location or the zero value; Lookup(name) also reports presence. MarshalJSON and UnmarshalJSON keep the serialized shape a name-keyed object, so output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
recordMapKeyLocations built a whole *Origin -- key Location, fields slice, sequences map -- purely to read its Fields, then copied that slice into a second one and discarded the rest. Nothing else holds the slice, so it can be sorted in place and handed over directly, removing one allocation per scalar-valued map.
./docs.sh output is checked in and verified by CI's git diff --exit-code; changing Origin.Fields's type and adding Get/Lookup changed the public API surface.
sort.Slice swaps through reflection; slices.SortFunc is generic and the package is already used elsewhere in the repo. Both call sites are name-ordering, one on the load path (recordMapKeyLocations) and one on the JSON round-trip. Lookup deliberately stays a hand-written loop. slices.IndexFunc reads better but its closure does not inline, so it pays a call per element: measured on 3/6/12 fields it is 5-100% slower on a hit and 2-3x slower on a miss, and misses dominate here since most fields carry no recorded location. The comment records that so it is not 'modernised' later.
reuvenharrison
force-pushed
the
perf/origin-fields-slice
branch
from
August 2, 2026 10:41
314575a to
db614a9
Compare
The obvious question on this change is why Fields moved off a map and Sequences did not. Two reasons, now written where a future reader will find them rather than only in the pull request. FieldLocations can drop the map because Location.Name already carries the key, so the map stored what the value repeated. In Sequences, Location.Name holds the item's value (an enum member, a required property) while the key is the field's name, so a slice would need a wrapper type invented purely to hold it -- trading a plain slice of an existing type for a slice of structs each holding a slice. The memory argument is also far weaker: only a collection with a sequence-valued field allocates one, measured at 5% of collections (20,000 of 390,004) on a 16.8 MB spec, and a nil map costs nothing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Origin.Fieldsismap[string]Location. A collection carries a handful of scalar fields, but a Go map allocates a whole bucket per collection whatever it holds, so these maps dominate the retained size of a parsed document.Profiling a 16.8 MB spec loaded with
IncludeOrigin,originFromSeq(where the inserts happen) allocated 339 MB, 13.9% of the total. With the slice it drops out of the top six entirely.What
FieldLocations, a slice in document order. EachLocationalready carries itsName, so the lookup key costs nothing extra, and a linear scan beats a map at these sizes.Get(name)returns a location or the zero value;Lookup(name)also reports presence.MarshalJSON/UnmarshalJSONkeep the serialized shape a name-keyed object, so output is unchanged.Lookupis a hand-written loop rather thanslices.IndexFuncon purpose: the closure does not inline, soIndexFuncpays a call per element. Measured on 3/6/12 fields it is 5-100% slower on a hit and 2-3x slower on a miss, and misses dominate (most fields carry no recorded location). A comment says so, to save the next reader from "modernising" it.sort.Slicebecomesslices.SortFuncat the two name-ordering sites, sincesort.Sliceswaps through reflection.recordMapKeyLocationsbuilt a whole*Originpurely to read itsFields, then copied that slice into a second one and discarded the rest. Nothing else holds it, so it is sorted in place and handed over.Sequencesdeliberately stays a map; the reasoning is on theOrigintype.Results
16.8 MB spec,
IncludeOrigin, median of 3 (peak from 7 runs):Retained is the target. A differ holds two documents resident for the duration of a comparison, so this halves the floor of the whole operation rather than of one load.
originFromSeqis the second-largest allocator, not the largest. The largest is the JSON decode, a separate concern tracked in getkin#1236.Breaking change
Origin.Fieldschanges type, soorigin.Fields[name]no longer compiles. The mechanical replacement isorigin.Fields.Lookup(name), same(Location, bool)shape.for rangeoverFieldsandlen(Fields)are unaffected.For scale: adapting oasdiff, a heavy consumer of this API, took 5 call sites and 13 test fixtures.